feat: add CraftDImage component for Android/KMP#103
feat: add CraftDImage component for Android/KMP#103rviannaoliveira wants to merge 6 commits intomainfrom
Conversation
- Add IMAGE_COMPONENT to CraftDComponentKey enum - Add CraftDContentScale enum (CROP, FIT, FILL_BOUNDS, FILL_WIDTH, FILL_HEIGHT, INSIDE, NONE) - Add ImageProperties data class with url, contentScale, contentDescription, actionProperties Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Compose: - Add toContentScale() extension mapping CraftDContentScale → ContentScale - Add CraftDImage composable with injectable imageLoader lambda - Add CraftDImageBuilder with injectable imageLoader constructor param XML: - Add CraftDImageComponent (AppCompatImageView wrapper) - Add CraftDImageComponentRender with injectable imageLoader - Register CraftDImageComponentRender in CraftDBuilderManager via optional imageLoader param Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Unit tests for ImageProperties serialization, toContentScale() and CraftDImageBuilder key - Updated docs/how-to-use/compose.md and view-system.md with CraftDImage usage examples - Registered CraftDImageBuilder (Coil) in Compose sample and CraftDImageComponentRender (Picasso) in XML sample - Added CraftDImage entry to dynamic.json - Switched app-sample-android to local craftd-xml project dependency - Added Coil 2.6.0 to libs.versions.toml - Deleted notes.md after context consumed Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
rviannaoliveira
left a comment
rviannaoliveira
left a comment
There was a problem hiding this comment.
Review — feat: add CraftDImage component for Android/KMP
Resumo
A implementação está bem estruturada — abstração do loader injetável, separação correta entre módulos, testes e docs presentes. Mas há dois bugs e três problemas que precisam de atenção antes do merge.
Crítico
1. contentScale ignorado silenciosamente
A propriedade contentScale de ImageProperties nunca é usada. A função toContentScale() foi criada mas não é chamada em lugar nenhum.
CraftDImage.kt: oimageLoaderrecebe(url, contentDescription, modifier)— semContentScaleCraftDImageComponentRender: não mapeiacontentScaleparaImageView.scaleType- O sample app passa
AsyncImagesemcontentScale
O resultado é que o servidor pode enviar "contentScale": "CROP" e o cliente ignora completamente. A assinatura do imageLoader precisa incluir ContentScale (Compose) ou um parâmetro equivalente.
2. @Stable/@Immutable de androidx.compose.runtime em commonMain — viola regra 4
CraftDContentScale e ImageProperties em craftd-core/commonMain importam androidx.compose.runtime.Stable e androidx.compose.runtime.Immutable. Essas são anotações de plataforma Compose, proibidas em commonMain.
Solução: remover as anotações (são hints de compilador Compose, desnecessárias em código de modelo de um módulo core) ou mover via expect/actual se o impacto de performance realmente justificar.
Moderado
3. Builder Compose não registrado — regra 9
A regra 9 do CLAUDE.md diz que todo novo builder deve ser registrado no CraftDBuilderManager. A justificativa de design (requer imageLoader) é válida, mas diverge da regra sem documentação explícita.
Sugestão: atualizar o CLAUDE.md para documentar essa exceção como padrão ("builders com dependências externas obrigatórias não são pré-registrados"), ou fornecer um imageLoader default no construtor que lance IllegalStateException com mensagem clara.
4. Testes de CraftDImageBuilderTest incompletos — task 4.3
A task 4.3 pedia: "verify imageLoader is called with correct args and actionProperties triggers listener". O teste atual só verifica a key. Falta:
- Verificar que o
imageLoaderé invocado comurlecontentDescriptioncorretos - Verificar que
listener.invoke(actionProperties)é chamado quandoactionProperties != null
Também não há testes para CraftDImageComponentRender (XML).
Positivo
- Abstração via lambda injetável está correta — cumpre regra 10 (sem acoplamento a Coil/Picasso)
onAction/fallback coberto tanto no Compose quanto no XMLImagePropertiesnocraftd-corecom serialização@Serializablecorreta- Docs de
compose.mdeview-system.mdatualizados com exemplos práticos - XML: registro condicional (
imageLoader?.let { ... }) é boa solução para oCraftDBuilderManager - Sample app demonstra ambas as plataformas (Coil no Compose, Picasso no XML)
- Instructions por plataforma em
.claude/instructions/são uma melhoria valiosa ao CLAUDE.md
Ação necessária antes do merge
- Corrigir a assinatura do
imageLoaderpara incluirContentScalee efetivamente usarproperties.contentScale - Remover
@Stable/@ImmutabledecommonMain(CraftDContentScaleeImageProperties) - Complementar os testes do
CraftDImageBuilderTest(imageLoader chamado + listener disparado)
| implementation("com.squareup.okhttp3:okhttp:4.9.1") | ||
| implementation("com.squareup.okhttp3:logging-interceptor:4.9.1") | ||
| implementation("com.squareup.picasso:picasso:2.8") | ||
| implementation(libs.coil.compose) |
| else -> Alignment.Top | ||
| } | ||
|
|
||
| internal fun CraftDContentScale?.toContentScale(): ContentScale = when (this) { |
There was a problem hiding this comment.
Extensão de nullable eu acho embaçado, acho que fica mais claro deixar o nullable check para o caller
| commonTest.dependencies { | ||
| implementation(kotlin("test")) | ||
| implementation(libs.kotlinx.serialization.json) | ||
| implementation(compose.runtime) |
|
|
||
| @Stable | ||
| @Immutable | ||
| enum class CraftDContentScale { |
There was a problem hiding this comment.
Não precisa de stable e immutable pra enum
| kotlinx-coroutines-test = "1.8.1" | ||
|
|
||
| # Coil | ||
| coil = "2.6.0" |
gabrielbmoro
left a comment
gabrielbmoro
left a comment
There was a problem hiding this comment.
Remover o coil e permitir que o dev use a biblioteca de image loading que quiser
2 participants
Summary
CraftDImagecomponent tocraftd-core,craftd-composeandcraftd-xmlImagePropertiesmodel comurl,contentScale,contentDescriptioneactionPropertiesCraftDContentScaleenum mapeando para ComposeContentScaleCraftDImageBuilder(Compose) comimageLoaderinjetável — não pré-registrado por designCraftDImageComponentRender(XML) comimageLoaderinjetável — registrado emCraftDBuilderManagerImageProperties,toContentScale()e key doCraftDImageBuildercompose.mdeview-system.md.claude/instructions/, regra deplatform:no/proposeTest plan
./gradlew :craftd-core:testDebugUnitTestpasses./gradlew :craftd-compose:testDebugUnitTestpasses./gradlew :app-sample-android:assembleDebugpasses🤖 Generated with Claude Code